sqlitevec: use DELETE by key instead of IN for virtual table deletes - #53
rossdonald wants to merge 2 commits into
Conversation
|
@dotnet-policy-service agree |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add active vec0 integration coverage, use one transaction for per-key deletes, and bump the provider version.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Replaces IN-based vec0 deletes with reusable per-key deletes to improve deletion performance.
Changes:
- Adds a parameterized single-key delete command.
- Applies per-key deletion to delete and upsert paths.
- Adds command-builder tests and updates SourceLink.
| File | Description |
|---|---|
MEVD/test/SqliteVec.UnitTests/SqliteCommandBuilderTests.cs |
Tests generated key-based delete SQL. |
MEVD/src/SqliteVec/SqliteCommandBuilder.cs |
Builds reusable parameterized delete commands. |
MEVD/src/SqliteVec/SqliteCollection.cs |
Uses per-key vector deletion. |
Directory.Packages.props |
Updates the SourceLink package version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| foreach (var key in keys) | ||
| { | ||
| keyParameter.Value = key; | ||
|
|
||
| await connection.ExecuteWithErrorHandlingAsync( |
There was a problem hiding this comment.
@rossdonald It sounds reasonable to address it. You could re-use the benchmarks copilot has created for me to measure the difference:
Details
// Benchmarks for https://github.com/CommunityToolkit/AI/issues/52
// Compares SqliteVec batch delete/upsert cost on the vec0 virtual table.
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Running;
using CommunityToolkit.VectorData.SqliteVec;
using Microsoft.Extensions.VectorData;
namespace SqliteVecBench;
public sealed class Record
{
[VectorStoreKey(StorageName = "chunk_id")]
public string ChunkId { get; set; }
[VectorStoreData]
public string Text { get; set; }
[VectorStoreVector(Dimensions: 256, DistanceFunction = DistanceFunction.CosineDistance)]
public ReadOnlyMemory<float> Embedding { get; set; }
}
[MemoryDiagnoser(displayGenColumns: false)]
public class SqliteVecDeleteBenchmarks
{
private const int BatchSize = 200;
private string _dbPath;
private SqliteCollection<string, Record> _collection;
private List<string> _missingKeys;
private List<string> _existingKeys;
private List<Record> _existingRecords;
[Params(10_000, 100_000)]
public int RowCount { get; set; }
private static ReadOnlyMemory<float> CreateVector(Random random)
{
float[] values = new float[256];
for (int i = 0; i < values.Length; i++)
{
values[i] = (float)random.NextDouble();
}
return new ReadOnlyMemory<float>(values);
}
[GlobalSetup]
public async Task SetupAsync()
{
_dbPath = Path.Combine(Path.GetTempPath(), $"sqlitevec-bench-{RowCount}-{Guid.NewGuid():N}.db");
_collection = new SqliteCollection<string, Record>($"Data Source={_dbPath}", "vec_chunks");
await _collection.EnsureCollectionExistsAsync();
Random random = new Random(42);
List<Record> batch = new List<Record>(1000);
for (int i = 0; i < RowCount; i++)
{
batch.Add(new Record { ChunkId = $"key-{i}", Text = "text", Embedding = CreateVector(random) });
if (batch.Count == 1000)
{
await _collection.UpsertAsync(batch);
batch.Clear();
}
}
if (batch.Count > 0)
{
await _collection.UpsertAsync(batch);
}
_missingKeys = Enumerable.Range(0, BatchSize).Select(i => $"missing-{i}").ToList();
// Keys spread across the table, from the "middle" of the key space.
_existingKeys = Enumerable.Range(0, BatchSize).Select(i => $"key-{i * (RowCount / BatchSize)}").ToList();
_existingRecords = _existingKeys
.Select(k => new Record { ChunkId = k, Text = "text", Embedding = CreateVector(random) })
.ToList();
}
[GlobalCleanup]
public void Cleanup()
{
_collection?.Dispose();
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
if (_dbPath is not null && File.Exists(_dbPath))
{
File.Delete(_dbPath);
}
}
// Delete a batch of keys that are not in the table: no rows are removed, so the
// benchmark is idempotent and measures the lookup cost only.
[Benchmark]
public Task DeleteBatch_MissingKeys() => _collection.DeleteAsync(_missingKeys);
// Single key delete for a key that is not present.
[Benchmark]
public Task DeleteSingle_MissingKey() => _collection.DeleteAsync("missing-0");
// Upsert of records that already exist: internally deletes the vector rows of the
// batch and re-inserts them, so the table size stays constant.
[Benchmark]
public Task UpsertBatch_ExistingRecords() => _collection.UpsertAsync(_existingRecords);
}
public static class Program
{
public static void Main(string[] args)
=> BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
}| "VectorDelete", | ||
| () => vectorDeleteCommand.ExecuteNonQueryAsync(cancellationToken), | ||
| cancellationToken).ConfigureAwait(false); | ||
| await DeleteVectorRowsAsync(connection, keys, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
@rossdonald please bump the version to 1.0.2-preview here:
(I am going to release a new version to nuget.org as soon as this PR gets merged)
adamsitnik
left a comment
There was a problem hiding this comment.
@rossdonald big thanks for your contribution!
Benchmarks show major perf wins (with only one regression):
Please address the remaining feedback, thank you!
| "VectorDelete", | ||
| () => vectorDeleteCommand.ExecuteNonQueryAsync(cancellationToken), | ||
| cancellationToken).ConfigureAwait(false); | ||
| await DeleteVectorRowsAsync(connection, keys, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
@rossdonald please bump the version to 1.0.2-preview here:
(I am going to release a new version to nuget.org as soon as this PR gets merged)
| foreach (var key in keys) | ||
| { | ||
| keyParameter.Value = key; | ||
|
|
||
| await connection.ExecuteWithErrorHandlingAsync( |
There was a problem hiding this comment.
@rossdonald It sounds reasonable to address it. You could re-use the benchmarks copilot has created for me to measure the difference:
Details
// Benchmarks for https://github.com/CommunityToolkit/AI/issues/52
// Compares SqliteVec batch delete/upsert cost on the vec0 virtual table.
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Running;
using CommunityToolkit.VectorData.SqliteVec;
using Microsoft.Extensions.VectorData;
namespace SqliteVecBench;
public sealed class Record
{
[VectorStoreKey(StorageName = "chunk_id")]
public string ChunkId { get; set; }
[VectorStoreData]
public string Text { get; set; }
[VectorStoreVector(Dimensions: 256, DistanceFunction = DistanceFunction.CosineDistance)]
public ReadOnlyMemory<float> Embedding { get; set; }
}
[MemoryDiagnoser(displayGenColumns: false)]
public class SqliteVecDeleteBenchmarks
{
private const int BatchSize = 200;
private string _dbPath;
private SqliteCollection<string, Record> _collection;
private List<string> _missingKeys;
private List<string> _existingKeys;
private List<Record> _existingRecords;
[Params(10_000, 100_000)]
public int RowCount { get; set; }
private static ReadOnlyMemory<float> CreateVector(Random random)
{
float[] values = new float[256];
for (int i = 0; i < values.Length; i++)
{
values[i] = (float)random.NextDouble();
}
return new ReadOnlyMemory<float>(values);
}
[GlobalSetup]
public async Task SetupAsync()
{
_dbPath = Path.Combine(Path.GetTempPath(), $"sqlitevec-bench-{RowCount}-{Guid.NewGuid():N}.db");
_collection = new SqliteCollection<string, Record>($"Data Source={_dbPath}", "vec_chunks");
await _collection.EnsureCollectionExistsAsync();
Random random = new Random(42);
List<Record> batch = new List<Record>(1000);
for (int i = 0; i < RowCount; i++)
{
batch.Add(new Record { ChunkId = $"key-{i}", Text = "text", Embedding = CreateVector(random) });
if (batch.Count == 1000)
{
await _collection.UpsertAsync(batch);
batch.Clear();
}
}
if (batch.Count > 0)
{
await _collection.UpsertAsync(batch);
}
_missingKeys = Enumerable.Range(0, BatchSize).Select(i => $"missing-{i}").ToList();
// Keys spread across the table, from the "middle" of the key space.
_existingKeys = Enumerable.Range(0, BatchSize).Select(i => $"key-{i * (RowCount / BatchSize)}").ToList();
_existingRecords = _existingKeys
.Select(k => new Record { ChunkId = k, Text = "text", Embedding = CreateVector(random) })
.ToList();
}
[GlobalCleanup]
public void Cleanup()
{
_collection?.Dispose();
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
if (_dbPath is not null && File.Exists(_dbPath))
{
File.Delete(_dbPath);
}
}
// Delete a batch of keys that are not in the table: no rows are removed, so the
// benchmark is idempotent and measures the lookup cost only.
[Benchmark]
public Task DeleteBatch_MissingKeys() => _collection.DeleteAsync(_missingKeys);
// Single key delete for a key that is not present.
[Benchmark]
public Task DeleteSingle_MissingKey() => _collection.DeleteAsync("missing-0");
// Upsert of records that already exist: internally deletes the vector rows of the
// batch and re-inserts them, so the table size stays constant.
[Benchmark]
public Task UpsertBatch_ExistingRecords() => _collection.UpsertAsync(_existingRecords);
}
public static class Program
{
public static void Main(string[] args)
=> BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
}

For the sqlite vector table, replace the slow IN operator with a loop using a statement that deletes by key.
Fixes: #52